Fixing compressor #34
Conversation
|
I just added softknee function to the compressor as kneeWidth arg in dB, 0 for hard knee I've tested it and it seems to work well el.compress(1, 100, -10, 100, 12, in, in);
el.compress(1, 100, -10, 4, 6, in, in);
el.compress(1, 100, -30, 10, 24, in, in);
Sorry for the cropped out axis, but everything seem in place from the values I can see |
|
@Mozoloa fantastic PR thank you! A couple of quick changes if you don't mind:
That sound ok? That feels to me like the right to navigate non-breaking changes and bug-fixes |
|
All good |
There was a problem hiding this comment.
Great @Mozoloa thank you again! Three more tiny things if you don't mind 🙏
js/packages/core/lib/dynamics.ts
Outdated
| @@ -44,22 +44,70 @@ export function compress( | |||
| xn: ElemNode, | |||
| ): NodeRepr_t; | |||
|
|
|||
| export function compress( | |||
| export function compress(a_, b_, c_, d_, e_, f_, g_?) { | |||
There was a problem hiding this comment.
It looks like this was probably a bug leftover from me, but if you don't mind fixing it while we're here I'd appreciate it–
The type signature above doesn't present an optional props argument up front, so we can write this function signature here as
export function compress(atkMs, relMs, threshold, ratio, sidechain, xn)
and then skip the whole let children = (typeof stuff below it
| * @param {Node} sidechain – sidechain signal to drive the compressor | ||
| * @param {Node} xn – input signal to filter | ||
| */ | ||
| export function skcompress( |
There was a problem hiding this comment.
Let's do the same thing here, remove that props argument here in the type signature, and then below let's do
export function skcompress(atkMs, relMs, threshold, ratio, kneeWidth, sidechain, xn)
and skip the whole optional arg stuff
README.md
Outdated
| [](https://github.com/elemaudio/elementary/blob/main/LICENSE.md) | ||
| [](https://discord.gg/xSu9JjHwYc) | ||
| [](https://www.npmjs.com/package/@elemaudio/core) |
There was a problem hiding this comment.
This is my bad I think– would you mind dropping the changes to README here so that the PR only affects the compressor code? I'll fix up the README changes in a follow-up commit
|
Ok got it, this will roll out with elem v3.1.0. Thank you @Mozoloa ! |
I've discovered that the compressor's ratio was behaving incorrectly. It was pretty much inversed in it's effect, a ratio of close to 50 was close to actually 1:1 in effect, same with close to 1 being actually close to infinity.
So I've redesigned the compressor to act in a more conventional way
I've tested it and so far it looks like it works alright, ratios are perfectly respected. More info in the code comments.
This might be app breaking for people that had implemented a hotfix on their end by reverse engineering the defective ratio tho.